Skip to content

fix(metrics): stop reporting Recall@K, F1@K and NDCG that measure nothing - #80

Open
rajarshidattapy wants to merge 1 commit into
supermemoryai:mainfrom
rajarshidattapy:fix/drop-degenerate-retrieval-metrics
Open

fix(metrics): stop reporting Recall@K, F1@K and NDCG that measure nothing#80
rajarshidattapy wants to merge 1 commit into
supermemoryai:mainfrom
rajarshidattapy:fix/drop-degenerate-retrieval-metrics

Conversation

@rajarshidattapy

Copy link
Copy Markdown

Fixes #67 — four of the six numbers under "RETRIEVAL QUALITY" were either duplicates or
non-standard quantities published under standard names.

Which option I took, and why

The issue offers two routes. I took the second one — stop emitting the degenerate metrics —
rather than wiring up LongMemEval's has_answer.

has_answer marks haystack messages, but retrieval metrics are computed over what a
provider returns: Supermemory memories, Mem0 memories, Zep graph edges and nodes, RAG chunks.
There is no uniform mapping from those artifacts back to source messages, so has_answer cannot
produce a trustworthy totalRelevant for the thing actually being scored — and LoCoMo and
ConvoMem don't ship equivalent labels at all, so it would only ever have worked for one
benchmark. Real relevance labels are a design change, not a bug fix; a wrong recall denominator
would be worse than none.

So this PR makes the reports honest now, and leaves the door open.

Changes

Dropped recallAtK, f1AtK, ndcg and totalRelevant from RetrievalMetrics /
RetrievalAggregates, the CLI report, the compare table, and the UI. Kept hitAtK,
precisionAtK, mrr, k and relevantRetrieved — all well defined without knowing the corpus.
calculateNDCG is deleted as dead code, and the reason is recorded on the type so the trio
doesn't get reintroduced by someone reading the field list and noticing gaps.

Judge failures no longer score as "not relevant". Both error paths in evaluateAllChunks
returned all-zero relevance, making a rate-limit or a timeout indistinguishable from "the
provider retrieved nothing useful" — quietly penalising the provider for the judge's problem.
They now return null, calculateRetrievalMetrics returns undefined, and the question is
excluded from the aggregate instead of contributing a zero. Both paths log a warning.

Note the distinction that had to be preserved: retrieving nothing is still a real zero
(hitAtK: 0, k: 0), because that's a measurement. Only judge failure is absent. Collapsing
those two would have swapped one silent bias for another.

Collapsed two duplicated tables. Removing three of seven columns from the compare table
meant editing the same width list in four border strings plus two row branches
(batch.ts), and three near-identical 12-line <td> blocks plus a 7-cell header
(compare/[compareId]/page.tsx). Both are now driven by a single column list, so header and body
cannot drift. That's where most of the −184 lines come from; it wasn't the goal but it was the
cheaper way to make the change correctly.

Tests

src/orchestrator/phases/retrieval-eval.test.ts drives the real code path against a stub
language model:

  • The reduced metric set is computed correctly, and asserts the four removed fields are absent
    from the output.
  • The bug, made concrete: "one relevant result at rank 1" and "four relevant results at ranks
    1–4" used to both score NDCG 1.0 and Recall 100%. The test shows Hit@K and MRR agree on those
    two cases (correctly — both hit, both rank a relevant result first) while Precision now
    separates them 25% vs 100%, which is the only honest signal available without labels.
  • A judge that throws, and a judge that answers unparseably, both yield no metrics rather than a
    zero.
  • An empty result set still yields a real zero.

The stub is hand-rolled (~10 lines) rather than ai/test's MockLanguageModelV2, which pulls in
msw — not a dependency here, and not worth adding for one test.

bun test 5/5, tsc --noEmit clean on src.

Compatibility

Report JSON and leaderboard rows written before this change still carry the removed fields; they
are simply ignored, so old reports render fine. New reports omit them, and the UI no longer reads
them, so there is no undefined-access path in either direction. No migration needed.

Anyone quoting Recall@K / F1@K / NDCG from a MemoryBench report should treat those figures as
withdrawn rather than changed — they were never measuring what their names implied.

Notes for the reviewer

  • src/orchestrator/phases/report.ts was already failing prettier --check at HEAD on a
    pre-existing basePromptTokens line. I left that line alone rather than bundle a reformat with
    a metrics change; every line I added is prettier-clean, and the other six files are fully clean.
  • ui/ has 4 pre-existing type errors (LeaderboardEntry.retrieval, promptTokens in
    question-list.tsx). I verified the error set is identical before and after this change, so
    none are mine — but they mean ui does not currently typecheck, worth its own fix.
  • Still broken, deliberately left alone: aggregateRetrievalMetrics sets k: m.k inside the
    reduce, so the aggregate k is whichever question came last rather than the run's k — reports
    can still print RETRIEVAL QUALITY (K=0). It's in a function this PR rewrites, but it's a
    separate defect with its own issue, and folding it in would blur what this change is
    accountable for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reported Recall@K, F1@K and NDCG are mathematically degenerate — they measure nothing beyond Hit@K

1 participant